Add picks variable selection to ae_oview module by osenan · Pull Request #331 · insightsengineering/teal.osprey

osenan · 2026-03-12T11:37:38Z

Pull Request

This partially closes #330

I based my changes in this PR.

Please compare both modules with this example app (requires to have teal.picks installed, osprey)

library("teal")
library("teal.picks")
devtools::load_all()

data <- teal_data() %>%
  within({
    library(dplyr)
    ADSL <- teal.data::rADSL
    ADAE <- teal.data::rADAE
    .add_event_flags <- function(dat) {
      dat <- dat %>%
        mutate(
          TMPFL_SER = AESER == "Y",
          TMPFL_REL = AEREL == "Y",
          TMPFL_GR5 = AETOXGR == "5",
          AEREL1 = (AEREL == "Y" & ACTARM == "A: Drug X"),
          AEREL2 = (AEREL == "Y" & ACTARM == "B: Placebo")
        )
      labels <- c(
        "Serious AE", "Related AE", "Grade 5 AE",
        "AE related to A: Drug X", "AE related to B: Placebo"
      )
      cols <- c("TMPFL_SER", "TMPFL_REL", "TMPFL_GR5", "AEREL1", "AEREL2")
      for (i in seq_along(labels)) {
        attr(dat[[cols[i]]], "label") <- labels[i]
      }
      dat
    }
    ADAE <- .add_event_flags(ADAE)
  })

join_keys(data) <- default_cdisc_join_keys[names(data)]

app <- init(
  data = data,
  modules = modules(
    tm_g_ae_oview(
      label = "Common AE (picks)",
      flag_var_anl = teal.picks::picks(
        teal.picks::datasets("ADAE"),
        teal.picks::variables(
           choices = c("TMPFL_SER", "TMPFL_REL", "TMPFL_GR5", "AEREL1", "AEREL2"),
          selected = "TMPFL_SER"
        )
      ),
      arm_var = teal.picks::picks(
        teal.picks::datasets("ADSL"),
        teal.picks::variables(
          choices = teal.picks::is_categorical(min.len = 2),
          selected = "ACTARMCD"
        )
      ),
      plot_height = c(600, 200, 2000)
    ),
    # Default method (old) for comparison
    tm_g_ae_oview(
      label = "Common AE (default)",
      dataname = "ADAE",
      flag_var_anl = teal.transform::choices_selected(
        selected = "AEREL1",
        choices = teal.transform::variable_choices(
          data[["ADAE"]],
          c("TMPFL_SER", "TMPFL_REL", "TMPFL_GR5", "AEREL1", "AEREL2")
        )
      ),
      arm_var = teal.transform::choices_selected(
        selected = "ACTARMCD",
        choices = c("ACTARM", "ACTARMCD")
      ),
      plot_height = c(600, 200, 2000)
    )
  )
)

shinyApp(app$ui, app$server)

I refactored the tm_g_ae_oview module to create a generic, keep the default for teal.transform and add a method for picks variable selection. The module works as expected and setting the variables is better with picks. The issue reported in the similar PR on the plot dimensions is fixed if we set the plot dimensions in the module. The ui of the picks ui components is different and it breaks, in my opinion, the color pattern of the teal app.

Here the teal.transform module

I added simple tests for the module as well

github-actions · 2026-03-12T11:37:58Z

🎉 Thank you for your contribution! Before this PR can be accepted, we require that you read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.

I have read the CLA Document and I hereby sign the CLA

osenan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

osenan · 2026-03-12T13:34:57Z

I have read the CLA Document and I hereby sign the CLA

osenan · 2026-03-12T15:00:17Z

Changed to draft as I need to fix:

Checks
Plot
Decorators

averissimo

Some preliminary comments.

I believe this is a good case for only keeping 1 version of the module logic. (We can convert directly from choices_selected -> variables())
I see that you're using picks(), consider using variables() instead for arm_var and flag_var_anl
- And then create picks() inside the tm_g_ae_oview() call

averissimo · 2026-03-12T16:38:41Z

R/tm_g_ae_oview.R

+                          arm_var = teal.picks::picks(
+                            teal.picks::datasets(),
+                            teal.picks::variables(
+                              choices = teal.picks::is_categorical(min.len = 2),
+                              selected = 1L
+                            )
+                          ),
+                          flag_var_anl = teal.picks::picks(
+                            teal.picks::datasets(),
+                            teal.picks::variables(
+                              choices = teal.picks::is_categorical(min.len = 2),
+                              selected = 1L
+                            )
+                          ),


I think this is a good candidate to use variables() in the arguments:

Suggested change

arm_var = teal.picks::picks(

teal.picks::datasets(),

teal.picks::variables(

choices = teal.picks::is_categorical(min.len = 2),

selected = 1L

)

),

flag_var_anl = teal.picks::picks(

teal.picks::datasets(),

teal.picks::variables(

choices = teal.picks::is_categorical(min.len = 2),

selected = 1L

)

),

arm_var = teal.picks::variables(

choices = teal.picks::is_categorical(min.len = 2),

selected = 1L

),

flag_var_anl = teal.picks::variables(

choices = teal.picks::is_categorical(min.len = 2),

selected = 1L

),

You can then overwrite arguments with picks() in tm_g_ae_oview.variables (and use dataname):

# ... arm_var <- teal.picks::picks(datasets(dataname), arm_var) flag_var_anl <- teal.picks::picks(datasets(dataname), flag_var_anl)

Changed, actually it is simpler good idea

DESCRIPTION

R/tm_g_ae_oview_picks.R

R/tm_g_ae_oview.R

R/tm_g_ae_oview_picks.R

averissimo · 2026-03-12T17:21:55Z

R/tm_g_ae_oview_picks.R

+      selectInput(
+        ns("arm_ref"),
+        "Control",
+        choices = NULL
+      ),
+      selectInput(
+        ns("arm_trt"),
+        "Treatment",
+        choices = NULL
+      ),


Interesting case here as we have dependent input fields from arm_var = picks(...).

Should these be created independently reusing arm_var datasets & variables

that is: having 2 picks

this will create problems downstream though

Or [possible feature]

Is this a case for having multiple values() in a picks, which generate multiple selectors.

picsk(datasets("ADAE"), variables(), values(), values()) # with some way to differentiate between control and treatement

@gogonzo maybe this is one of the cases of bookmark manager we remember.

I think this is related to this issue: insightsengineering/teal.picks#14.
My initial plan was to go for the second option. But it is hard to distinguish here what is what: should picks(datasets("ADAE"), variables(), values(), values()) join all the values in a single selection or not? To disambiguate I thought about using operators: picks(datasets("ADAE"), variables()) + values() | values(): a picks with two options/selectors.

Currently I think it is better if we create a function to handle those cases.

Let's solve the issue and then update this module

R/tm_g_ae_oview_picks.R

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

osenan · 2026-03-16T11:42:32Z

I have read the CLA Document and I hereby sign the CLA

osenan · 2026-03-17T06:46:27Z

The plot and decorator I will handle in a separate issue finally

github-actions · 2026-03-17T06:55:35Z

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  ---------------------
R/tm_g_ae_oview_picks.R       274     210  23.36%   80-84, 89-93, 162-397
R/tm_g_ae_oview.R              33       0  100.00%
R/tm_g_ae_sub.R               288     288  0.00%    60-379
R/tm_g_butterfly.R            371     371  0.00%    125-534
R/tm_g_decorate.R              42      42  0.00%    17-91
R/tm_g_events_term_id.R       271     271  0.00%    63-364
R/tm_g_heat_bygrade.R         293     293  0.00%    136-459
R/tm_g_patient_profile.R      691     691  0.00%    159-913
R/tm_g_spiderplot.R           322     322  0.00%    100-464
R/tm_g_swimlane.R             345     345  0.00%    127-525
R/tm_g_waterfall.R            429     429  0.00%    109-590
R/utils.R                      85      80  5.88%    31-73, 107-240
R/zzz.R                         3       3  0.00%    4-7
TOTAL                        3447    3345  2.96%

Diff against main

Filename                   Stmts    Miss  Cover
-----------------------  -------  ------  --------
R/tm_g_ae_oview_picks.R     +274    +210  +23.36%
R/tm_g_ae_oview.R           -192    -225  +100.00%
TOTAL                        +82     -15  +2.81%

Results for commit: 5f0cfc2

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

github-actions · 2026-03-17T06:55:42Z

Unit Tests Summary

1 files 2 suites 0s ⏱️
8 tests 8 ✅ 0 💤 0 ❌
20 runs 20 ✅ 0 💤 0 ❌

Results for commit 5f0cfc2.

♻️ This comment has been updated with latest results.

github-actions · 2026-03-17T06:55:46Z

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
tm_g_ae_oview	👶		$+0.12$	$+7$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	$±Time$	Test Case
tm_g_ae_oview	👶	$+0.03$	tm_g_ae_oview_argument_verification_fails_when_arm_var_is_choices_selected_but_flag_var_anl_is_pick
tm_g_ae_oview	👶	$+0.02$	tm_g_ae_oview_argument_verification_fails_when_arm_var_is_pick_but_flag_var_anl_is_choices_selected
tm_g_ae_oview	👶	$+0.05$	tm_g_ae_oview_module_creation_creates_a_teal_module_using_choices_selected_default_method_
tm_g_ae_oview	👶	$+0.02$	tm_g_ae_oview_module_creation_creates_a_teal_module_using_picks_.pick_method_

Results for commit 8195974

♻️ This comment has been updated with latest results.

osenan · 2026-03-17T10:14:47Z

Now the example should be tested with this app:


library(teal)
devtools::load_all()
library(teal.picks)

data <- teal_data() %>%
   within({
     library(dplyr)
     ADSL <- rADSL
     ADAE <- rADAE
     .add_event_flags <- function(dat) {
       dat <- dat %>%
         mutate(
           TMPFL_SER = AESER == "Y",
           TMPFL_REL = AEREL == "Y",
           TMPFL_GR5 = AETOXGR == "5",
           AEREL1 = (AEREL == "Y" & ACTARM == "A: Drug X"),
           AEREL2 = (AEREL == "Y" & ACTARM == "B: Placebo")
         )
       labels <- c(
         "Serious AE", "Related AE", "Grade 5 AE",
         "AE related to A: Drug X", "AE related to B: Placebo"
       )
       cols <- c("TMPFL_SER", "TMPFL_REL", "TMPFL_GR5", "AEREL1", "AEREL2")
       for (i in seq_along(labels)) {
         attr(dat[[cols[i]]], "label") <- labels[i]
       }
       dat
     }
     ADAE <- .add_event_flags(ADAE)
   })

 join_keys(data) <- default_cdisc_join_keys[names(data)]

 ADAE <- data[["ADAE"]]

 app <- init(
   data = data,
   modules = modules(
     tm_g_ae_oview(
       label = "AE Overview",
       dataname = "ADAE",
       arm_var = teal.picks::variables(
          choices = tidyselect::starts_with("ACTARM"),
          selected = "ACTARMCD"
      ),
      flag_var_anl = teal.picks::variables(
          choices = c("TMPFL_SER", "TMPFL_REL", "TMPFL_GR5", "AEREL1", "AEREL2"),
          selected = "AEREL1"
      ),
      plot_height = c(600, 200, 2000)
     )
   )
 )
 if (interactive()) {
   shinyApp(app$ui, app$server)
 }

averissimo

Awesome! It works great and it even supports picks conversion for choices_selected

R/tm_g_ae_oview.R

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

osenan and others added 4 commits March 12, 2026 12:14

feat: add S3 method dispatch for picks or teal transform variables

060314d

tests: add test for tm_g_ae_oview module

975ae0a

chore: fix spelling

b6fd8f6

chore: fix sign credentials

d68f94a

osenan added the core label Mar 12, 2026

osenan requested a review from a team March 12, 2026 11:38

[skip style] [skip vbump] Restyle files

3dd972e

osenan mentioned this pull request Mar 12, 2026

[Feature Request]: Allow modules to use teal.picks #330

Open

3 tasks

osenan marked this pull request as draft March 12, 2026 14:59

averissimo reviewed Mar 12, 2026

View reviewed changes

averissimo mentioned this pull request Mar 13, 2026

[Feature Request]: Implement picks on modules insightsengineering/teal.modules.clinical#1472

Open

osenan and others added 4 commits March 16, 2026 12:31

Update DESCRIPTION

89ecbea

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

Update R/tm_g_ae_oview_picks.R

630f5bf

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

Update R/tm_g_ae_oview_picks.R

2869ce9

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

Update R/tm_g_ae_oview_picks.R

c2f07a3

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

osenan added 5 commits March 16, 2026 12:53

chore: update docs after fix style

bf70fff

chore: fix typo in description

88fbf7e

refactor: change defaults on ae_oview generic and use dataname directly

8bd9bd3

refactor: add only necessary arguments to the UI

584c88b

fix: update test after refactor on method

b3f3eb0

osenan marked this pull request as ready for review March 17, 2026 06:45

github-actions bot added 2 commits March 17, 2026 06:46

[skip style] [skip vbump] Restyle files

e00faee

[skip roxygen] [skip vbump] Roxygen Man Pages Auto Update

440b9fe

chore: fix linters and examples

5abc2ca

github-actions bot and others added 4 commits March 17, 2026 10:15

[skip style] [skip vbump] Restyle files

df0f269

chore: fix R CMD check and lintr

b7219f0

[skip style] [skip vbump] Restyle files

640f80e

chore: fix styler and linter

6e6e222

osenan requested a review from averissimo March 17, 2026 11:56

averissimo requested changes Mar 17, 2026

View reviewed changes

R/tm_g_ae_oview.R Outdated Show resolved Hide resolved

R/tm_g_ae_oview.R Outdated Show resolved Hide resolved

R/tm_g_ae_oview.R Show resolved Hide resolved

averissimo self-assigned this Mar 17, 2026

osenan and others added 6 commits March 17, 2026 15:31

chore: revert to magrittr pipe

73c6f60

chore: remove pipe consistency linter as we need magrittr pipes

8ca1bc1

Update R/tm_g_ae_oview.R

0e4d1bc

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

refactor: use as.picks to reuse picks srv and ui in choices_selected

af68495

chore: fix lintr comment to avoid automatic restyling

e5e9dad

Update R/tm_g_ae_oview_picks.R

5f0cfc2

Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com> Signed-off-by: Oriol Senan <35930244+osenan@users.noreply.github.com>

osenan requested a review from averissimo March 18, 2026 11:33

Uh oh!

Conversation

osenan commented Mar 12, 2026

Pull Request

Uh oh!

github-actions bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osenan commented Mar 12, 2026

Uh oh!

osenan commented Mar 12, 2026

Uh oh!

averissimo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

averissimo Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osenan Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

averissimo Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

llrs-roche Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

osenan Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

osenan commented Mar 16, 2026

Uh oh!

osenan commented Mar 17, 2026

Uh oh!

github-actions bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

github-actions bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

github-actions bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Performance Difference

Uh oh!

osenan commented Mar 17, 2026

Uh oh!

averissimo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 12, 2026 •

edited

Loading

averissimo left a comment •

edited

Loading

averissimo Mar 12, 2026 •

edited

Loading

github-actions bot commented Mar 17, 2026 •

edited

Loading

github-actions bot commented Mar 17, 2026 •

edited

Loading

github-actions bot commented Mar 17, 2026 •

edited

Loading